Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix multiple tests with refactoring driver for duplicate class. #16853

Merged
merged 9 commits into from
Jul 5, 2024
Merged

Fix multiple tests with refactoring driver for duplicate class. #16853

merged 9 commits into from
Jul 5, 2024

Conversation

hernanmd
Copy link
Collaborator

@hernanmd hernanmd commented Jul 2, 2024 •
edited

Fix a couple of tests failing, reported in #16850, due to missing support to pre-configure refactorings for test purposes.
Also configured missing mocks for the preview presenter.
Fix also for #16825

@hernanmd hernanmd mentioned this pull request Jul 2, 2024
Closed
@MarcusDenker
Copy link
Member

Argh, I did merge the revert in the meantime so now this is a conflict.

@MarcusDenker
Copy link
Member

I am doing a revert of the revert: #16854

@hernanmd
Copy link
Collaborator Author

hernanmd commented Jul 2, 2024

I am doing a revert of the revert: #16854

Thanks, I will check further fails and check better next time.
(and I forgot to mention I was working on this issue)

@MarcusDenker MarcusDenker reopened this Jul 2, 2024
@MarcusDenker
Copy link
Member

Revert fixed the conflicts, tests are now run

MarcusDenker
MarcusDenker requested changes Jul 2, 2024
View reviewed changes
Copy link
Member

@MarcusDenker MarcusDenker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this adds one more failing testNoUncategorizedMethods

@hernanmd hernanmd self-assigned this Jul 2, 2024
Hernán Morales Durand added 2 commits July 2, 2024 21:46
Fix testThatThereAreNoSelectorsRemainingThatAreSentButNotImplemented …
cfb418a 
…in ReClassForGeneratingEqualAndHashExistingImplementors
Merge 6c2b670
5296bf9
Hernán Morales Durand added 2 commits July 3, 2024 08:48
@unfurl-links unfurl-links bot mentioned this pull request Jul 3, 2024
Closed
@MarcusDenker
Copy link
Member

Now load fails:

MetacelloNotification: Loaded -> Refactoring-Changes --- tonel:///builds/workspace/est_and_branch_Pipeline_PR-16853/64/src()
Error: Package Refactoring-Core depends on the following classes:
  ReCopyClassRefactoring
You must resolve these dependencies before you will be able to load these definitions: 
  ReCopyClassRefactoring>>#classMethods
  ReCopyClassRefactoring>>#classMethods:
  ReCopyClassRefactoring>>#copyClass
  ReCopyClassRefactoring>>#copyClass:withName:
  ReCopyClassRefactoring>>#copyMethods:of:in:
  ReCopyClassRefactoring>>#instanceMethods

@MarcusDenker MarcusDenker added the Status: Need more work The issue is nearly ready. Waiting some last bits. label Jul 3, 2024
@carolahp carolahp mentioned this pull request Jul 3, 2024
Closed
@MarcusDenker
Copy link
Member

Now all the test that the revert fixed are failing:


ReDuplicateClassDriverTest.testDuplicateClass	
 
 
 osx-64 / Tests-osx-64 / MacOSX64.Refactoring.Transformations.Tests.RBCopyPackageParametrizedTest.testCopyPackageAndChangesCopyReferences(#rbClass->RBCopyPackageRefactoring)	6.8 sec	1
 osx-64 / Tests-osx-64 / MacOSX64.Refactoring.Transformations.Tests.RBCopyPackageParametrizedTest.testCopyPackageWithParameters(#rbClass->RBCopyPackageRefactoring)

In addition, we have:

ReleaseTest>>testUndeclared

It might fix other, but honestly, I am starting to get lost completely

Might it not be a better idea to revert ?

@guillep
Copy link
Member

guillep commented Jul 4, 2024

Hi Marcus, I agree with you. We should revert. That was the rule we set.

@hernanmd it is important for us that this gets fixed fast, either by reverting or by fixing it.
It simplifies the management of all the other ~60 PRs we have.

@hernanmd
Copy link
Collaborator Author

hernanmd commented Jul 4, 2024 •
edited

this adds one more failing testNoUncategorizedMethods

Hi @MarcusDenker, could you share a link to see the failed tests?
I also got lost in the issues here.

@MarcusDenker
Copy link
Member

MarcusDenker commented Jul 5, 2024 •
edited by unfurl-links bot

The failing tests are here:

https://ci.inria.fr/pharo-ci-jenkins2/job/Test%20pending%20pull%20request%20and%20branch%20Pipeline/job/PR-16853/8/testReport/

Add rules to ignore critics in TestCase data classes.
0aca021 
Add wrongly deleted methods and instance variables in ReCopyClassRefactoring.
Re-add wrongly deleted SycCmDuplicateClassCommand.
@MarcusDenker MarcusDenker removed the Status: Need more work The issue is nearly ready. Waiting some last bits. label Jul 5, 2024
@MarcusDenker
Copy link
Member

Thanks!

@MarcusDenker MarcusDenker merged commit e87057e into pharo-project:Pharo13 Jul 5, 2024
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MarcusDenker MarcusDenker MarcusDenker approved these changes

@jecisc jecisc Awaiting requested review from jecisc

Assignees

@hernanmd hernanmd

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@hernanmd @MarcusDenker @guillep